kvm: Enable virtio drivers based on guest os display name #3879

ustcweizhou · 2020-02-11T10:19:52Z

Description

When we add new guest os, sometimes we missed the records in guest_os_hypervisor.
However, the guest disk model (virtio/ide) is determined by record in the table.
It causes the issue that some new guest os(eg Debian 8/9) uses e1000 instead of virtio nic, and ide disk instead of virtio disk.

To fix the issue permanantly, pass the guest os name in guest_os if the record for kvm is not found in guest_os_hypervisor.

Related commit:7ac9f00eeeb4cd37ec39efeba066e799b581b1a0

Types of changes

Breaking change (fix or feature that would cause existing functionality to change)
New feature (non-breaking change which adds functionality)
Bug fix (non-breaking change which fixes an issue)
Enhancement (improves an existing feature and functionality)
Cleanup (Code refactoring and cleanup, that may add test cases)

Screenshots (if appropriate):

How Has This Been Tested?

svenvogel · 2020-02-11T22:22:53Z

@weizhouapache can you explain it me a little bit better. if i understand it correctly if guestosmapping and host is null then we set guestOS.getDisplayName(). why display name? maybe i stand on the line 😕

weizhouapache · 2020-02-11T22:29:45Z

@weizhouapache can you explain it me a little bit better. if i understand it correctly if guestosmapping and host is null then we set guestOS.getDisplayName(). why display name? maybe i stand on the line 😕

@svenvogel
here is an example:
in guest_os, we have Debian 9

mysql> select id,name,display_name from guest_os where id in (289,290);
+-----+------+-----------------------------+
| id  | name | display_name                |
+-----+------+-----------------------------+
| 289 | NULL | Debian GNU/Linux 9 (32-bit) |
| 290 | NULL | Debian GNU/Linux 9 (64-bit) |
+-----+------+-----------------------------+
2 rows in set (0.01 sec)

however, in guest_os_hypervisor, there is no record for these guest oses on kvm

mysql> select * from guest_os_hypervisor where guest_os_id in (289,290) and hypervisor_type='KVM';
Empty set (0.00 sec)

when we start a vm with Debian 9, in StartCommand it shows

"arch":"x86_64","os":"Debian GNU/Linux 9 (64-bit)","platformEmulator":"Other"

so vm is started with ide disk and e1000 nic.

with this change, "platformEmulator" will be set to same value as "os"

yadvr · 2020-02-13T05:50:35Z

Some testing required - @DaanHoogland @andrijapanicsb

andrijapanicsb · 2020-02-13T10:50:31Z

@blueorangutan package

blueorangutan · 2020-02-13T10:51:14Z

@andrijapanicsb a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2020-02-13T11:08:29Z

Packaging result: ✖centos6 ✔centos7 ✔debian. JID-856

yadvr · 2020-02-14T06:31:09Z

server/src/main/java/com/cloud/hypervisor/KVMGuru.java

        }
        if (guestOsMapping == null || host == null) {
-            to.setPlatformEmulator("Other");
+            to.setPlatformEmulator(guestOS.getDisplayName());


@ustcweizhou how about we check and fallback to "Other" in case guestOS.getDisplayName() is null or empty (unlikely but just to make code more defensive)?

yadvr

LGTM, left a minor remark. Did not test it.

yadvr · 2020-02-14T06:32:10Z

@blueorangutan test

blueorangutan · 2020-02-14T06:32:18Z

@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

DaanHoogland · 2020-02-14T15:26:07Z

sorry @weizhouapache, i editted according to @rhtyd 's comment (kinda, revert if you want)

weizhouapache · 2020-02-14T16:58:54Z

@DaanHoogland thanks Daan !
I changed unknown to Other

yadvr · 2020-02-17T15:41:04Z

@blueorangutan package

blueorangutan · 2020-02-17T15:41:33Z

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2020-02-17T16:36:48Z

Packaging result: ✖centos6 ✔centos7 ✔debian. JID-887

DaanHoogland · 2020-02-17T16:41:33Z

@blueorangutan test

blueorangutan · 2020-02-17T16:42:33Z

@DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

blueorangutan · 2020-02-18T00:54:46Z

Trillian test result (tid-1015)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 28098 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3879-t1015-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
Smoke tests completed. 76 look OK, 1 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File
test_02_vpc_privategw_static_routes	`Failure`	190.76	test_privategw_acl.py
test_03_vpc_privategw_restart_vpc_cleanup	`Failure`	187.20	test_privategw_acl.py
test_04_rvpc_privategw_static_routes	`Failure`	269.84	test_privategw_acl.py

yadvr · 2020-02-19T07:38:09Z

Tests LGTM, the failures are known issue

shwstppr

LGTM based on code changes and travis

When we add new guest os, sometimes we missed the records in guest_os_hypervisor. However, the guest disk model (virtio/ide) is determined by record in the table. It causes the issue that some new guest os(eg Debian 8/9) uses e1000 instead of virtio nic, and ide disk instead of virtio disk. To fix the issue permanantly, pass the guest os name in guest_os if the record for kvm is not found in guest_os_hypervisor. Related commit:7ac9f00eeeb4cd37ec39efeba066e799b581b1a0

kvm: Enable virtio drivers based on guest os display name

7ea0e90

weizhouapache requested review from PaulAngus and yadvr February 11, 2020 10:21

weizhouapache added Severity:BLOCKER component:kvm labels Feb 11, 2020

weizhouapache closed this Feb 11, 2020

weizhouapache reopened this Feb 11, 2020

weizhouapache closed this Feb 11, 2020

weizhouapache reopened this Feb 11, 2020

yadvr added this to the 4.13.1.0 milestone Feb 13, 2020

yadvr reviewed Feb 14, 2020

View reviewed changes

yadvr approved these changes Feb 14, 2020

View reviewed changes

fall back if null

d6fae5a

use Other instead unknown

8d69839

yadvr requested review from DaanHoogland, andrijapanicsb and nvazquez February 18, 2020 04:28

shwstppr approved these changes Feb 19, 2020

View reviewed changes

yadvr merged commit 37d2b85 into apache:4.13 Feb 19, 2020

kvm: Enable virtio drivers based on guest os display name #3879

kvm: Enable virtio drivers based on guest os display name #3879

Uh oh!

Conversation

ustcweizhou commented Feb 11, 2020

Description

Types of changes

Screenshots (if appropriate):

How Has This Been Tested?

Uh oh!

svenvogel commented Feb 11, 2020

Uh oh!

weizhouapache commented Feb 11, 2020

Uh oh!

yadvr commented Feb 13, 2020

Uh oh!

andrijapanicsb commented Feb 13, 2020

Uh oh!

blueorangutan commented Feb 13, 2020

Uh oh!

blueorangutan commented Feb 13, 2020

Uh oh!

yadvr Feb 14, 2020

Choose a reason for hiding this comment

Uh oh!

yadvr left a comment

Choose a reason for hiding this comment

Uh oh!

yadvr commented Feb 14, 2020

Uh oh!

blueorangutan commented Feb 14, 2020

Uh oh!

DaanHoogland commented Feb 14, 2020

Uh oh!

weizhouapache commented Feb 14, 2020

Uh oh!

yadvr commented Feb 17, 2020

Uh oh!

blueorangutan commented Feb 17, 2020

Uh oh!

blueorangutan commented Feb 17, 2020

Uh oh!

DaanHoogland commented Feb 17, 2020

Uh oh!

blueorangutan commented Feb 17, 2020

Uh oh!

blueorangutan commented Feb 18, 2020

Uh oh!

yadvr commented Feb 19, 2020

Uh oh!

shwstppr left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants